r? @bluss

(rust_highfive has picked a reviewer for you, use r? to override)

I think these methods also make more sense than the to_be and to_le function from a type-system point of view: It doesn't make sense to repeatedly call to_be on an integer, but you can, the same way that it doesn't make sense to repeatedly call encode() on a string in Python 2, but you can do so there (which will raise an error for non-ASCII, but still).

CC #49792

Very fair point! Three different method classes makes sense.

Hi! I don't know if I'm supposed to add my 2 cents here, but since I can, I will.

I've been using the to_bytes kind of transformation without regards for the endianness. This is because this is what my program is doing:

• Receive an N-Dimensional Vec<f32>
• Take its (non-randomly-seeded) u64 hash
• Take the bytes of that hash
• Feed those bytes to a pseudorandom generator as its seed.

I'm using this to make a generalized N-D Perlin (and in the future, Simplex) noise library. Since I'm using the hash's bytes as the seed for the pseudorandom generator, I didn't care about endianness when I implemented it.

But now that I think of it, specifying the endianness would allow me to have the exact same noise patterns regardless of the platform in which the library is running. So I think, even in my case, this is a better feature than the recently merged to_bytes :)

Thanks for your hard work 🙇‍♂️

Python implements a similar API on integers:

 |  from_bytes(...) from builtins.type
 |      int.from_bytes(bytes, byteorder, *, signed=False) -> int
 |      
 |      Return the integer represented by the given array of bytes.
 |      
 |      The bytes argument must be a bytes-like object (e.g. bytes or bytearray).
 |      
 |      The byteorder argument determines the byte order used to represent the
 |      integer.  If byteorder is 'big', the most significant byte is at the
 |      beginning of the byte array.  If byteorder is 'little', the most
 |      significant byte is at the end of the byte array.  To request the native
 |      byte order of the host system, use `sys.byteorder' as the byte order value.
 |      
 |      The signed keyword-only argument indicates whether two's complement is
 |      used to represent the integer.
 |  
 |  to_bytes(...)
 |      int.to_bytes(length, byteorder, *, signed=False) -> bytes
 |      
 |      Return an array of bytes representing an integer.
 |      
 |      The integer is represented using length bytes.  An OverflowError is
 |      raised if the integer is not representable with the given number of
 |      bytes.
 |      
 |      The byteorder argument determines the byte order used to represent the
 |      integer.  If byteorder is 'big', the most significant byte is at the
 |      beginning of the byte array.  If byteorder is 'little', the most
 |      significant byte is at the end of the byte array.  To request the native
 |      byte order of the host system, use `sys.byteorder' as the byte order value.
 |      
 |      The signed keyword-only argument determines whether two's complement is
 |      used to represent the integer.  If signed is False and a negative integer
 |      is given, an OverflowError is raised.

(Note that the byteorder argument is mandatory.)

This was discussed briefly at @rust-lang/libs triage yesterday but the conclusion was that we're going to leave comments individually here if we feel so.

I personally would not want to merge this PR. I think that leveraging the already-stable to_le and to_be methods to work in combination with to_bytes and from_bytes (already implemented) is the best way to work with these methods. Given that to_le and to_be are already stable I wouldn't feel we need to stabilize to_le_bytes or to_be_bytes. The question then to me is whether we need to include "native" in the name. I feel that the fact that these are native-endianness is implied by the lack of other information in the method name as otherwise we'd specify big or little.

@rfcbot fcp close

Let's see what others think though!

Team member @alexcrichton has proposed to close this. The next step is review by the rest of the tagged teams:

• @Kimundi
• @SimonSapin
• @alexcrichton
• @dtolnay
• @sfackler
• @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

The question then to me is whether we need to include "native" in the name. I feel that the fact that these are native-endianness is implied by the lack of other information in the method name as otherwise we'd specify big or little.

As you can see in this thread, the implied native isn't clear for everyone. In fact, I think one should actively think about whether one wants a big-endian, little-endian or native byte representation for the integer. Having to_le and to_be on the integers does not achieve this, as you can just omit calls to them and never notice. If you instead have the choice between to_le_bytes, to_be_bytes, to_native_bytes, it is very clear that you have to make a choice, and it's immediately documented, too. If you see to_bytes in the wild, you wouldn't really know whether the author intended to make the code platform-specific or just forgot to add to_be or to_le.

This alternative was discussed in #49792 and I mildly preferred the current design in order to avoid the multiplication of API surface. This PR proposes replacing two methods with six.

I don’t really mind changing, especially since to_bytes and from_bytes were only stabilized in this cycle (1.29) so we can still remove them. However the motivation based only on principle feels a bit weak.

@tbu-, do you feel that this change would prevent bugs? What do you think of the "intermediate" option of renaming the existing methods but not adding the be/le ones? (Keep documenting them as to be combined with to_le and to_be.)

@tbu-, do you feel that this change would prevent bugs?

Yes, this is the intention behind this change. As you can see above, we already caught one with the PR alone. :)

What do you think of the "intermediate" option of renaming the existing methods but not adding the be/le ones?

I think that would be worse than what we currently have, as to_be().to_native_bytes() looks wrong, as we're combining a big-endian function with a function operating on native endian.

I think the optimization based on number of functions is misguided.

In C, zero-terminated "strings" share the same type, whether they're encoded as UTF-8 or just treated as bag of bytes. Rust makes the distinction because it is useful to prevent errors, it makes sense to make the user cast between bytes and UTF-8-strings, even if this results in an enormous method duplication between byte slices and strings.

As shown with the Python example, other languages consider the endianness of the integers part of the encoding process, too.

I think I'm personally just not sold on this. I see the cost of six functions, some of which duplicate the functionality of existing functions, as not worth it. Dealing with endianness is inherently tricky and I don't feel like making all the methods longer and more wordy will really reduce the trickiness, but rather just make it less ergonomic to call when you already know which one you'd like.

I see the cost of six functions, some of which duplicate the functionality of existing functions, as not worth it.

I understand this part.

don't feel like making all the methods longer and more wordy will really reduce the trickiness, but rather just make it less ergonomic to call when you already know which one you'd like.

I don't really understand this part. It's

to_be().to_bytes()              → to_be_bytes()            [reduction by  5 chars]
to_le().to_bytes()              → to_le_bytes()            [reduction by  5 chars]
to_bytes()                      → to_native_bytes()        [increase  by  7 chars]
i32::from_be(i32::from_bytes()) → i32::from_be_bytes()     [reduction by 10 chars]
i32::from_le(i32::from_bytes()) → i32::from_le_bytes()     [reduction by 10 chars]
i32::from_bytes()               → i32::from_native_bytes() [increase  by  7 chars]

To me, it looks like it decreases the ergonomics of doing the platform-specific thing slightly but also increases the ergonomics of doing the platform-independent thing slightly, and the important part: It brings them to the same level. Currently, doing the platform-independent thing is slightly less ergonomic.

Rust wants to encourage good code, if we want to bring ergonomics of the methods into play, then I think an even playing field between platform-independent methods and platform-dependent methods is in line with Rust's objectives.

I feel that "native" in a name doesn’t really work on its own. It only sort of does in "native endian", and even then only by opposition to little/big endian.

What do you all think of transmute_to_bytes and transmute_from_bytes, to express that this is a type-level change without a memory representation change?

Also the more I think about it the more I’m becoming partial to having explicit {to,from}_{le,be}_bytes conversions. The {to,from}_{le,be} methods have the merit of already being stable, but they’re kinda weird in taking and returning the same type. In fact the implementation of to_be and from_be for example is identical! I understand that they’re intended to be thought about differently, and perhaps their use is self-documenting what direction of conversion is intended, but it feels like the type system would be better suited to this (even though adding a whole series of dedicated types like struct BigEndianU32(u32); is probably not worth it).

@tbu- to me ergonomics isn't literally how many characters you type but also how easy it is to remember idioms, and remembering that there's endian conversions on integers plus byte<-> int conversions means it's easy to remember how to compose. With everything it can be confusing about knowing which one was the one implemented and there's now a choice to be made of which to do in various circumstances.

@SimonSapin I think we could try to find a better name, yeah, but I'd reserve transmute for unsafe-like operations and would prefer to not have it here on a safe method.

I am today starting on my first Rust program. I need to read a binary file format. When reading and writing files or network data, the endianness is an inherent part of the format's or protocol's definition.

It seems far more complicated to me to first read an incorrect intermediate value using host endianness and then adjust it to the correct value with bit manipulations. Byte-swapping is a completely unnecessary complication when we can just read it correctly in the first place.

I would really appreciate it if conversions of the form 'u32::from_le_bytes([u8; 4])' were made available.

This looks great, thanks! Sorry for one last hoop to jump through, could you change the tracking issue in #[unstable] attributes to #52963 ? r=me with that.

@SimonSapin Done.

@bors r+

Thanks!

📌 Commit 0ddfae5 has been approved by SimonSapin

⌛ Testing commit 0ddfae5 with merge 215bf3a...

💥 Test timed out

I would really like to see this be put into a trait rather than functions.

As a trait this could be implemented by a builtin derive which would make it work for structures composed of primitive types like this is being implemented on currently.

This would allow for simple creation of packed (or C structures by ignoring padding) structures in Rust (like an IP header or binary file header) and then the automatic conversion to and from the raw bytes on the wire/file.

I have a library to do this now and I use it in many of my projects. I think the ability to be able to handle reading and writing packed structures safely from bytestreams is critical enough to a systems language to justify it being put into the core language rather than being an external library. Especially given it would be almost no code increase to the Rust base (would be a procmacro), and would have no implications to programs unless they decide to use the feature.

@gamozolabs I think it would be appropriate to publish that library on crates.io, garner adoption and prove out the API. At that point, someone could consider writing an RFC, but I think it's probably immature to jump to that right now.

@BurntSushi will this affect byteorder at all?

@jonhoo That's a pretty broad question! Nothing immediately pops out at me. byteorder should continue to work fine?

Hehe, that's true. I guess I was more wondering whether byteorder will transition to use these methods instead of having its own implementation?

Hehe, that's true. I guess I was more wondering whether byteorder will transition to use these methods instead of having its own implementation?

Oh! Hah. That didn't cross my mind at all because byteorder is one of the most conservative crates I maintain, and I won't increase its MSRV for something like this.